Skip to content

Refactor AcceptHeaderLocaleResolver to implement LocaleContextResolver - #29239

Closed
vpavic wants to merge 3 commits into
spring-projects:mainfrom
vpavic:improve-locale-resolvers
Closed

Refactor AcceptHeaderLocaleResolver to implement LocaleContextResolver#29239
vpavic wants to merge 3 commits into
spring-projects:mainfrom
vpavic:improve-locale-resolvers

Conversation

@vpavic

@vpavic vpavic commented Sep 30, 2022

Copy link
Copy Markdown
Contributor

This PR wraps up efforts to improve org.springframework.web.servlet.LocaleResolver hierarchy started in:

The core change proposed here (first commit) is to refactor AcceptHeaderLocaleResolver to implement LocaleContextResolver, which makes the all locale resolvers provided by Spring MVC LocaleContextResolver based. It also aligns Spring MVC with Spring WebFlux, where all resolvers work with LocaleContext rather than plain Locale.

The two subsequent commits are optional, but could be helpful as they prepare ground to simplifying LocaleResolver hierarchy in the future:

  • second commit deprecates AbstractLocaleResolver in favor of AbstractLocaleContextResolver, as nothing is using AbstractLocaleResolver after the first commit
  • third commit is a bit more substantial as it deprecates org.springframework.web.servlet.LocaleResolver and reworks all the references to operate against LocaleContextResolver - while technically a breaking change, this only impacts those with custom LocaleResolver that don't implement LocaleContextResolver

At present, AcceptHeaderLocaleResolver is the only LocaleResolver
implementation that doesn't also implement LocaleContextResolver. This
is also not aligned with WebFlux, where the entire resolver hierarchy
works with LocaleContext.

This commit updates AcceptHeaderLocaleResolver to implement
LocaleContextResolver.
This commit deprecates AbstractLocaleResolver in favor of
AbstractLocaleContextResolver.
This commit deprecates LocaleResolver and updates all references to use
LocaleContextResolver instead.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 30, 2022
@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jan 24, 2023
@jhoeller

Copy link
Copy Markdown
Contributor

While I see the point of the PR, I am not inclined to proceed with phasing out LocaleResolver which has been around for 20 years. AcceptHeaderLocaleResolver implementing LocaleContextResolver does not provide any user-level benefits on its own (since the accept-header strategy is only capable of providing a plain locale to begin with). From my perspective, this would really have go along with a deprecation of LocaleResolver as per your third commit, and that's what I try not to inflict on the two-decade-long usage this has in configuration points, customizations/decorations, and also in testing scenarios.

Point taken that WebFlux has full-scale LocaleContext resolution. However, since that is not the only difference that shows MVC's age over WebFlux, that level of SPI style alignment between MVC and WebFlux is not a goal per se... as long as the overall capabilities are equivalent.

@jhoeller jhoeller closed this Nov 23, 2023
@jhoeller jhoeller added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 23, 2023
@vpavic
vpavic deleted the improve-locale-resolvers branch April 1, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants